Skip to content

iOS: stop presenting a suspended app's last reading as current (#147) - #253

Merged
BenSeverson merged 2 commits into
mainfrom
claude/fix-147-background-staleness
Jul 30, 2026
Merged

iOS: stop presenting a suspended app's last reading as current (#147)#253
BenSeverson merged 2 commits into
mainfrom
claude/fix-147-background-staleness

Conversation

@BenSeverson

Copy link
Copy Markdown
Owner

Addresses two of the three fix directions on #147. #147 stays open for the third — see the bottom, it's the part that actually closes the safety gap and it's a decision rather than a patch.

staleDate on Live Activity content

The app declares no background modes and requests the activity with pushType: nil, so iOS suspends the WebSocket within seconds of backgrounding and nothing can update the activity until the app is reopened. With staleDate: nil, the lock screen kept presenting that last temperature as current, indefinitely — across an 8–12 hour firing.

For a number an operator may act on, wrong and confident is worse than visibly out of date. Now 90 s: the firmware broadcasts every second so any longer gap is already abnormal, but it's loose enough to ride out a Wi-Fi blip while foregrounded without flickering to stale.

The terminal content keeps staleDate: nil on purpose — "Complete" isn't a reading that decays, and ageing it would make a finished firing look like a lost connection.

Resync on foreground

scenePhase == .active now refreshes status over REST and then restarts the socket. Previously the socket came back dead while connectionState still read .connected, so returning to the app mid-firing showed the reading from whenever the phone went into a pocket, until the user happened to pull to refresh.

Two deliberate details:

  • REST before reconnect. The snapshot is what makes the screen correct immediately; reconnecting alone leaves it wrong until the next broadcast.
  • resumeIfConnected() is a no-op unless a connection was already established. A foreground event must not start dialling a kiln the user never connected to, or paper over an .error they still need to read.

Not fixed — and this is the important part

The "Kiln Error / check kiln immediately" notification still requires the app to be running. That intersection — app backgrounded enough for the notification gate to allow it, but foregrounded enough for the socket to be alive — is nearly empty, which is the safety gap the issue is really about. Nothing here changes that.

Closing it means one of:

  • wiring the firmware's existing webhookUrl setting to a push bridge (ntfy or similar), or
  • an APNs relay for ActivityKit push tokens — much larger, and odd for a LAN-only device.

Both are infrastructure choices with ongoing cost and privacy implications. Not mine to pick, so I've left #147 open rather than closing it on a partial fix and letting the gap look addressed.

Verification

Not compiled — this session is a Linux container with no Swift toolchain, so CI's macOS build-ios job is the only compile check. Staleness rendering and scenePhase transitions both need a device to confirm behaviourally; I've checked call sites and actor isolation statically, which is not the same thing.


Generated by Claude Code

Two of the three fix directions on the issue. The third is a product
decision, not a patch, and is left alone deliberately.

**staleDate on Live Activity content.** The app declares no background
modes and requests the activity with `pushType: nil`, so iOS suspends the
WebSocket within seconds of backgrounding and nothing can update the
activity until the app is opened again. With `staleDate: nil` the lock
screen kept presenting that last temperature as current, indefinitely,
across an 8-12 hour firing. For a number an operator may act on, wrong
and confident is worse than visibly out of date. 90s: the firmware
broadcasts every second, so any longer gap is already abnormal, but it is
loose enough to ride out a Wi-Fi blip while foregrounded without
flickering to stale.

The terminal content keeps `staleDate: nil` on purpose — "Complete" is
not a reading that decays, and ageing it would make a finished firing
look like a lost connection.

**Resync on foreground.** `scenePhase == .active` now refreshes status
over REST and then restarts the socket. Previously the socket came back
dead while `connectionState` still read `.connected`, so returning to the
app mid-firing showed the reading from whenever the phone went into a
pocket until the user happened to pull to refresh.

REST before reconnect, because the snapshot is what makes the screen
correct immediately; reconnecting alone leaves it wrong until the next
broadcast. `resumeIfConnected()` is a no-op unless a connection was
already established, so a foreground event cannot start dialling a kiln
the user never connected to, or paper over an `.error` they still need to
read.

**Not fixed: background delivery.** The "Kiln Error / check kiln
immediately" notification still requires the app to be running, which is
the safety gap the issue is really about. Closing it means either wiring
the firmware's existing webhookUrl to a push bridge, or an APNs relay for
ActivityKit push tokens — infrastructure choices for a LAN-only device,
not something to pick unilaterally. #147 stays open for that.

Not verified by compilation: no Swift toolchain on Linux. CI's build-ios
job is the only compile check, and staleness rendering plus scenePhase
transitions need a device to confirm.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PyJ47o5sWvpb9vMk1RNKkb

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1594c9648b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +91 to +92
webSocket.disconnect()
webSocket.connect(host: host, port: port, apiToken: apiToken)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Prevent the canceled socket from reconnecting again

Whenever resumeIfConnected() runs while the old receive loop is still awaiting a message, disconnect() cancels that socket and connect() immediately sets shouldReconnect back to true. The canceled loop then reaches receiveLoop's catch and schedules another reconnect; when that fires it cancels the replacement socket, whose loop schedules the next reconnect, producing continuous socket churn and missed updates after foregrounding. Suppress reconnects from the canceled generation or wait for the old receive loop to finish before enabling reconnection.

Useful? React with 👍 / 👎.


Task {
await activity.update(.init(state: state, staleDate: nil))
await activity.update(.init(state: state, staleDate: Self.staleDate()))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Render a distinct state when ActivityKit marks content stale

When the app remains suspended beyond 90 seconds, staleDate only causes ActivityKit to set ActivityViewContext.isStale; it does not replace the displayed state automatically. The lock-screen and Dynamic Island views still render currentTemp, status, and remaining time identically and never inspect context.isStale, so the old temperature continues to look current indefinitely despite this timestamp. Update both Live Activity presentations to visibly label or hide stale readings.

Useful? React with 👍 / 👎.

Comment thread ios/Bisque/Bisque/ContentView.swift Outdated
guard phase == .active else { return }
Task {
if let client = connection.apiClient {
await store.refreshAll(using: client)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Process status transitions during the foreground snapshot

When a firing reaches complete, error, or idle while the app is suspended, this call enters refreshAll, which assigns previousStatus = status.status without invoking handleStatusTransition; the next WebSocket frame has the same status and therefore cannot recover the skipped transition. As a result, the existing Live Activity is never ended and peakTempC is not reset, so its old high-water mark can contaminate the next firing's reported peak. Apply the transition handling before replacing previousStatus during this foreground resync.

Useful? React with 👍 / 👎.

Comment thread ios/Bisque/Bisque/ContentView.swift Outdated
guard phase == .active else { return }
Task {
if let client = connection.apiClient {
await store.refreshAll(using: client)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Apply the status snapshot independently of auxiliary requests

On foregrounding, this waits for refreshAll, but that method only applies the fetched status after the profiles and settings requests also succeed. If either unrelated endpoint fails or hangs, a successful current status is discarded and the WebSocket restart is postponed until the structured child requests finish, potentially leaving the suspended temperature on screen for the API client's 30-second resource timeout. Fetch and apply the status independently, or reconnect the live feed without waiting for the auxiliary refreshes.

Useful? React with 👍 / 👎.

… churn

Four review findings, three of them P1 and two of them introduced by the
first pass of this PR.

**staleDate was inert.** Setting it only flips `ActivityViewContext.isStale`;
ActivityKit does not alter the rendering. Neither the lock screen nor the
Dynamic Island read that flag, so the frozen temperature went on looking
exactly as current as before — the PR claimed a behaviour change that did
not happen. Both presentations now dim the reading and, where there is
room, say why. The countdown is *replaced* rather than dimmed: "~2h
remaining" extrapolated from a frozen frame is a claim, not a stale
observation.

**The foreground resync caused continuous socket churn.** disconnect()
cancels the receive loop's task and socket, but the loop reaches its catch
block regardless (`Task.isCancelled` guards loop entry, not the catch) and
hops to the main actor to schedule a reconnect — by which time connect()
has already set shouldReconnect back to true. That reconnect then cancels
the healthy socket, whose loop schedules the next one, at 1-8s intervals,
indefinitely. Worse than the bug being fixed.

Each openConnection() now stamps a generation; a loop reports its socket
lost only while its own stamp is still current. Cancelling the task alone
would not do: openConnection() can supersede a loop that was never
cancelled. disconnect() bumps it too, so an explicit disconnect silences
its loop by generation rather than relying on shouldReconnect.

**A transition that happened while suspended was swallowed.** refreshAll
assigned previousStatus directly, so a firing that completed in the
background was adopted without running handleStatusTransition — and the
next frame carried the same status, making the difference undetectable
forever. The Live Activity was never ended and peakTempC carried its
high-water mark into the next firing's completion notification: the same
lie #146 was about, by a different route. Snapshots now run the
transition, gated on hasObservedStatus so the first load after launch does
not announce a firing the app never watched.

That gate is applied to the WebSocket path too, where the same spurious
announcement was possible on a first frame of "complete".

**Status no longer waits on unrelated requests.** refreshAll awaited
status, profiles and settings together, so a failure or stall on either of
the latter discarded a good status and — on foreground — held the socket
restart behind it. Status is now its own method; the foreground path calls
that and reconnects, leaving profiles and settings to connect time and
pull-to-refresh.

Still not compiled: no Swift toolchain on Linux, so CI's build-ios job
remains the only compile check.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PyJ47o5sWvpb9vMk1RNKkb

Copy link
Copy Markdown
Owner Author

All four correct, and two of them were introduced by this PR. Fixed in 8a99e10.

P1 — staleDate was inert. The sharpest of the four: setting it only flips context.isStale, and neither presentation read that flag, so the frozen temperature went on looking exactly as current as before. The PR claimed a behaviour change that did not happen. Both the lock screen and all four Dynamic Island presentations now dim the reading and, where there is room, say why.

One judgement call: the countdown is replaced rather than dimmed. "~2h remaining" derived from a frozen frame is a claim, not a stale observation — dimming it would still be asserting something the app has no basis for.

P1 — continuous socket churn, caused by this PR. Also correct, and worse than the bug I was fixing. disconnect() cancels the loop's task, but the catch block runs anyway (Task.isCancelled guards loop entry), and the main-actor hop lands after connect() has already restored shouldReconnect. That reconnect then cancels the healthy socket, whose loop schedules the next — self-sustaining at 1–8 s intervals.

Fixed with a generation stamp per openConnection(); a loop reports its socket lost only while its stamp is current. Worth noting cancellation alone would not have been sufficient, as the comment says: openConnection() can supersede a loop that was never cancelled, and that loop's Task.isCancelled is false. disconnect() bumps the generation too, so it doesn't depend on shouldReconnect staying false.

P1 — swallowed transition. Correct, including the consequence: because the next frame carries the same status, the difference becomes undetectable permanently, so the Live Activity is never ended and peakTempC contaminates the next firing's completion notification — the same lie #146 was about, by another route. Snapshots now run the transition.

Guarded on a new hasObservedStatus, because previousStatus starts at "idle" and is indistinguishable from an observed idle: without it, the first load after launch would announce a firing that ended before the app ever ran. I applied that gate to the WebSocket path as well, where a first frame of "complete" could do the same thing.

P2 — status coupled to unrelated requests. Correct. Split into refreshStatus, and the foreground path calls that and reconnects; profiles and settings stay on connect and pull-to-refresh.

Still not compiled — no Swift toolchain here, so build-ios remains the only compile check on this.


Generated by Claude Code

@BenSeverson
BenSeverson merged commit 79ab64e into main Jul 30, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants